Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

New template for Alcatel AOS: show lldp remote-system #1917

Merged
merged 19 commits into from
Dec 9, 2024
Merged

New template for Alcatel AOS: show lldp remote-system #1917

merged 19 commits into from
Dec 9, 2024
+381 −0

Conversation

evilmonkey19
Copy link
Contributor

@evilmonkey19 evilmonkey19 commented Nov 28, 2024 •
edited
Loading

It has support for R6 and R8 versions

@mjbear
Copy link
Contributor

mjbear commented Nov 29, 2024

Looks like there are some capture group names to normalize.
It's going take some time to analyze. Thank you.

evilmonkey19 and others added 2 commits November 29, 2024 09:51
@evilmonkey19 @mjbear
Update ntc_templates/templates/alcatel_aos_show_lldp_remote-system.te…
2feeb4d 
…xtfsm

Co-authored-by: Michael Bear <[email protected]>
@mjbear
Fix up white space matching
2f78897 
* Remove actual space (and supplant with \s+)
* Replace \s* with more proper \s+ where there will be one or more
  spaces
@mjbear
Copy link
Contributor

mjbear commented Nov 29, 2024

@evilmonkey19
Why are so many of the capture group regexes .*?

Have you found things such as REMOTE_ID or the MAC address to be empty?

There's some weirdness from vendors' implementation of LLDP (I've seen that before as well) when it comes to what shows in some of those fields ... but not all of those fields from the looks of it.

@mjbear
Copy link
Contributor

mjbear commented Nov 29, 2024 •
edited
Loading

@evilmonkey19
Edit: Please disregard as this is already handled. Kudos!

Do we really want or care about a System (Neighbor) Description that contains (null)?

It may make sense to exclude that string and just bring back an empty string instead. This will avoid capturing data we (probably) don't care about and make things easier to programmatically check (ex: with Python boolean).

Edit2: I was seeing (null) in port description within the YAML and that's what I meant. Oh well, I'm going to handle that one.

@mjbear
Copy link
Contributor

mjbear commented Nov 29, 2024

@evilmonkey19
Do we care about all possible Capabilities or just those that are enabled?

One example is the IOS or NXOS template where only the enabled capabilities matter so they're the only ones captured.

@mjbear mjbear mentioned this pull request Nov 29, 2024
Merged
@mjbear
Copy link
Contributor

mjbear commented Nov 29, 2024

Submitted evilmonkey19/pull/3 to @evilmonkey19's fork.

Following normalized examples in https://github.com/networktocode/ntc-templates/blob/master/docs/dev/data_model.md

Alcatel AOS Normalized
PORT LOCAL_INTERFACE
CHASSIS CHASSIS_ID
MAC_PORT MAC_ADDRESS
REMOTE_ID ?
CHASSIS_SUBTYPE ?
PORT_SUBTYPE ?
PORT_DESCRIPTION NEIGHBOR_INTERFACE
SYSTEM_NAME NEIGHBOR_NAME
SYSTEM_DESCRIPTION NEIGHBOR_DESCRIPTION
CAPABILITIES_SUPPORTED question posed in /pull/1917
CAPABILITIES_ENABLED question posed in /pull/1917
MANAGEMENT_IP_ADDRESS MGMT_ADDRESS
MED_DEVICE_TYPE ?
MED_CAPABILITIES ?
MED_EXTENSION_TLVS_PRESENT ?
MED_POWER_TYPE POWER_DEVICE_TYPE
MED_POWER_SOURCE POWER_SOURCE
MED_POWER_PRIORITY POWER_PRIORITY
MED_POWER_VALUE POWER_REQUESTED
REMOTE_PORT_MAC_PHY_AUTONEG ?
MAU_TYPE ?

@evilmonkey19
Copy link
Contributor Author

@evilmonkey19 Why are so many of the capture group regexes .*?

Have you found things such as REMOTE_ID or the MAC address to be empty?

There's some weirdness from vendors' implementation of LLDP (I've seen that before as well) when it comes to what shows in some of those fields ... but not all of those fields from the looks of it.

The main reason for such many .* it is not sure that the output will appear

@evilmonkey19
Merge pull request #3 from mjbear/aos_lldp_remote-sys_pr1917
fa3feb6 
Updates, fixes, and normalization to Alcatel AOS LLDP
@evilmonkey19
Copy link
Contributor Author

I merged your PR in my repo because it looks great! I have tried against real data even 👍

@jmcgill298 jmcgill298 merged commit b6b030d into networktocode:master Dec 9, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjbear mjbear mjbear left review comments

@jmcgill298 jmcgill298 jmcgill298 approved these changes

@itdependsnetworks itdependsnetworks Awaiting requested review from itdependsnetworks itdependsnetworks is a code owner

@jvanderaa jvanderaa Awaiting requested review from jvanderaa jvanderaa is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@evilmonkey19 @mjbear @jmcgill298